Skip to content

chore(avm)!: Bytecode Retrieval pre-audit docs #20718

Merged
MirandaWood merged 11 commits intomerge-train/avmfrom
mw/avm-bc-ret-pre-audit-docs
Feb 27, 2026
Merged

chore(avm)!: Bytecode Retrieval pre-audit docs #20718
MirandaWood merged 11 commits intomerge-train/avmfrom
mw/avm-bc-ret-pre-audit-docs

Conversation

@MirandaWood
Copy link
Contributor

@MirandaWood MirandaWood commented Feb 20, 2026

Bytecode retrieval pre-audit PR including:

  • New comments/documentation
  • Rearranging code
  • Renaming variables/relations

This trace does a lot of delegation, so the pre audit has been completed assuming the target circuits constrain as expected! The actual columns used in these lookups have been checked though.

Closes AVM-54

TODOs/Notes: See comments for those to discuss (otherwise, I need to add preconditions, doc on why we zero some properties on error, and tracegen tests Update: complete!).

@MirandaWood MirandaWood force-pushed the mw/avm-bc-ret-pre-audit-docs branch from c3592ae to fd65309 Compare February 23, 2026 13:05
@MirandaWood MirandaWood marked this pull request as ready for review February 23, 2026 13:06
@MirandaWood MirandaWood force-pushed the mw/avm-bc-ret-pre-audit-docs branch from fd65309 to 82a758d Compare February 25, 2026 14:23
Copy link
Contributor

@jeanmon jeanmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good work overall!

// We convert the bytecode to a shared_ptr because it will be shared by some events.
auto shared_bytecode = std::make_shared<std::vector<uint8_t>>(std::move(klass.packed_bytecode));
// Emits BytecodeDecompositionEvent.
decomposition_events.emit({ .bytecode_id = bytecode_id, .bytecode = shared_bytecode });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we emit "decomposition events" suggest that the circuit does not map simulation in an equivalent way. bc_retrieval.pil does not have interaction with bc_decomposition.pil IIRC.
Did you reason about the completeness at "higher level" of the mechanism of "bc retrieval, fetching, decomposition, hashing"? It looks like it is not fully modular and "local review" might not be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes, in terms of the 'map' I added to the top:

 *  execution.pil --> bc_retrieval.pil --> contract_instance_retrieval.pil --> nullifier_check.pil
 *                                                                         --> address_derivation.pil
 *                                                                         --> update_check.pil
 *                                     --> retrieved_bytecodes_tree_check.pil
 *                                     --> class_id_derivation.pil
 *                --> instr_fetching.pil --> bc_decomposition.pil <-> bc_hashing.pil
 *                                                                --> precomputed.pil

The lookup execution.pil --> bc_retrieval.pil occurs on the first row of each context (sel_first_row_in_context) regardless of any errors. The instruction fetching lookup occurs whenever we have sel_bytecode_retrieval_success which IIRC is a 'subset' of sel_first_row_in_context (i.e. it's only on at the first row of each context, but not vice versa).
I think that the decomposition event emission in simulation maps 1:1 to the circuit behaviour since we emit it when:

  • we have already emitted a retrieval event (i.e. can't have a decomp event without a retrieval event)
  • there is no error, otherwise we return early (sel_bytecode_retrieval_success == 0 in this case)
  • we have already accessed this bytecode (is_new_class == 0 and a decomp event will have already been emitted in this case)

I do think at the instruction fetching level this is hard to reason about, since in the circuit we link instr_fetching.pil --> bc_decomposition.pil but in simulation we 'link' bytecode_manager (=> bc_retrieval, bc_decomposition, bc_hashing) to the decomp event. It's the higher level 'link' at simulation's execute which puts together instruction fetching and bytecode management. So it works in a very roundabout way.

Maybe for the future we should consider moving emitting the bytecode decomp event via the instruction fetching call, to make it easier to reason about?

@MirandaWood MirandaWood requested a review from jeanmon February 27, 2026 10:06
@MirandaWood MirandaWood force-pushed the mw/avm-bc-ret-pre-audit-docs branch from ad46454 to 701e9ed Compare February 27, 2026 10:20
Copy link
Contributor

@jeanmon jeanmon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I have only minor suggestions.

@MirandaWood MirandaWood merged commit 57f2542 into merge-train/avm Feb 27, 2026
10 checks passed
@MirandaWood MirandaWood deleted the mw/avm-bc-ret-pre-audit-docs branch February 27, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants